fix(schema): turn the deserializer's silent sinks into named errors - #161
Merged
Merged
Conversation
Nine confirmed audit findings on the JSON contract. Each is a silent sink: a
legitimate value — usually the one CSS would have you write — swallowed
without a word, producing a wrong render instead of an error. An LLM does not
read serde's output; it sets a field, nothing objects, and it concludes the
field was honoured.
- `AnimatedBackground`'s hand-written `Deserialize` had three sinks, not the
two the audit named. An unknown `preset` fell through `_ =>` to
`gradient_shift` with no colors; `zones` parsed with
`.ok().unwrap_or_default()`; and — the third, found by reading the
function — `colors` and `gradient_type` did the same *even with a
correctly spelled preset*. The symptom is the worst available: an entirely
black video, no diagnostic. A correctly spelled `heropattern` in the flat
legacy form also fell through to `gradient_shift`. Every branch now routes
through a typed struct with error propagation.
- `Edges` accepted any object at all: `{"padding": {"horizontal": 20}}`
deserialized to zero on all four sides. `CssStyle`'s `deny_unknown_fields`
gave the illusion of protection, but the enum one level down had four
defaulted fields and no guard of its own.
- `border-radius` per-corner was the only composite in `CssStyle` using
snake_case. The kebab form every neighbour uses failed, fell to another
variant, and produced radius 0. Both spellings now work, unknown ones are
reported.
- `width: "max-content"` was unreachable: `Length`'s own string catch-all
absorbed it before the `Keyword` variant was tried, so the box collapsed
to 0. `LineHeight` had the identical defect with `"normal"` — found by
sweeping the other fifteen untagged enums in the repo, which is now the
complete list: no catch-all-before-specific ordering remains.
- Animation `property` was a free `String` on three types, so an unknown or
wrongly-cased name made the animation inert. Now constrained at the schema
layer only — the solver is untouched — with a did-you-mean when the sole
difference is the naming convention.
- The exported JSON schema declared `background` invalid on `Scene` and
`View`, because `deny_unknown_fields` emits `additionalProperties: false`
and `background` was `schemars(skip)`. The repo's own examples failed
against the schema the generators consume: 31 violations across 6 files.
- A literal `$` in any string was fatal if and only if the document happened
to contain a `config` block — a price, or `$PATH` in a terminal, blocked
by an unrelated key elsewhere. The scan now always runs, and warns rather
than rejecting: a declared variable can never survive `merge_variables`
unresolved, so anything the scan still finds is by definition outside the
declared set. An override naming an undeclared variable stays a hard
error.
- `PositionMode::Named` accepted any string while only `"absolute"` does
anything, so `"position": "relative"` — legitimate CSS — dropped `x`/`y`
in silence. It warns now, deduplicated per distinct value: `prepare_scene`
re-runs this `Deserialize` over the whole tree once per frame, so an
unguarded warning would print over a thousand times on a 1200-frame
render.
Also completes PR #158's hardening at the level below: `deny_unknown_fields`
on `Animation` and `Keyframe`, the keys inside a `keyframes[*]`.
Tests: full workspace green on this branch alone; every example still
validates, and now also validates against the exported schema.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Round 4, vague B — lot schéma & serde, 9 constats vérifiés.
Chacun est un puits silencieux : une valeur légitime — souvent celle que le CSS ferait écrire — avalée sans un mot, produisant un rendu faux plutôt qu'une erreur. Un LLM ne lit pas la sortie de serde : il pose un champ, rien ne proteste, il en conclut que c'est pris en compte.
AnimatedBackgroundzones, etcolors/gradient_type→ vidéo entièrement noire, aucun diagnostic.Edges(padding/margin){"padding": {"horizontal": 20}}→ zéro sur les quatre faces.border-radiuspar coinpropertyd'animationbackgrounddéclaré invalide surScene/View: 31 violations sur 6 fichiers — les propres exemples du dépôt, contre le schéma que les générateurs consomment.width: "max-content"$littéralconfig.position: "relative"x/yjetés sans un signal.Le troisième puits du fond noir
Le brief nommait deux puits et demandait de trouver le troisième en lisant la fonction. Il existe :
colorsetgradient_typeétaient parsés en.ok().unwrap_or_default()même avec un preset correctement orthographié. Uncolorsabsent ou malformé produisait donc un dégradé vide — écran noir — sur un scénario par ailleurs irréprochable.Trouvaille supplémentaire du même passage : un
heropatternbien écrit en forme legacy plate tombait aussi dans le bras fourre-tout et devenait ungradient_shift.Chaque bras route désormais par un struct typé avec propagation d'erreur.
La classe de bug des enums
untaggedwidth: "max-content"était injoignable parce que le fourre-toutStringdeLengthl'absorbait avant que le variantKeywordne soit tenté. Question posée en ouvert dans le brief : combien d'autres ?Les 15 enums
untaggeddu dépôt ont été balayés. Deux étaient touchés :Size(le constat) etLineHeight, où"normal"subissait exactement le même sort puis se résolvait à 0. Les deux sont corrigés. Aucun restant — les autres sont soit correctement ordonnés, soit disjoints par forme JSON.Deux avertissements plutôt que deux rejets, et pourquoi
$littéral. Le scan tourne désormais systématiquement — l'incohérence venait de ce qu'il ne s'exécutait que dans la brancheconfig— mais il avertit au lieu de rejeter. Un rejet dur casserait tout document légitime contenant un$(un prix, un$PATHdans un terminal) dès qu'il gagne un blocconfig. Par construction, une variable réellement déclarée ne peut pas survivre àmerge_variablessans être résolue : ce que le scan trouve encore est donc, par définition, hors du périmètre déclaré. Le rejet dur pour un override référençant une variable non déclarée — cas sans ambiguïté — est inchangé.position."relative"et"static"restent du CSS légitime, et un rejet casserait un usage volontaire (sortir du flux sans coordonnées). Avertissement donc — dédoublonné par valeur :prepare_sceneréexécute ceDeserializesur l'arbre entier une fois par frame, un avertissement non gardé sortirait plus de mille fois sur un rendu de 1200 frames. Chaque valeur fautive distincte garde sa ligne, une seule fois. Un test épingle ce garde.Complète la PR #158 à l'étage du dessous
deny_unknown_fieldssurAnimationetKeyframe— les clés à l'intérieur d'unkeyframes[*], que la vague A n'avait pas le droit de toucher.Reste ouvert
Contre le schéma complet exporté par la CLI (qui fusionne en plus l'union
Component), il reste 6 violations sur 3 fichiers, toutes sans rapport avecbackgroundet présentes avant cette PR : un enfant valide au runtime ne matche pas leoneOfdeComponent. Je vérifie et j'ouvre une issue séparée.Vérification
cargo test --workspacesur cette branche seule : 21 cibles, 0 échec.clippy -D warningsetfmt --checkpropres. Les 8 exemples : 7 valident (le huitième est l'issue #157, préexistante àmain), et les 8 valident désormais contre le schéma exporté parrustmotion-core, contre 31 violations avant.